Skip to content

Support updated insert geometry - #2890

Open
aidenwu23 wants to merge 5 commits into
mainfrom
pr/insert-recon-update
Open

Support updated insert geometry#2890
aidenwu23 wants to merge 5 commits into
mainfrom
pr/insert-recon-update

Conversation

@aidenwu23

@aidenwu23 aidenwu23 commented Aug 23, 2026

Copy link
Copy Markdown

Briefly, what does this PR introduce? Please link to any relevant presentations or discussions.

This PR updates the insert reconstruction to support the latest mechanical design introduced in: eic/epic#1153.

What is the urgency of this PR?

  • High (please describe reason below)
  • Medium
  • Low

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Optimization (issue #__)
  • Updated parameters, constants (issue #__)
  • Updated documentation
  • other: __

Please check if any of the following apply

  • This PR requires changes to geometry (epic PR: __)
  • This PR requires changes to EDM4eic (EDM PR: __)
  • This PR introduces breaking changes. Please describe changes users need to make below.
  • This PR changes default behavior. Please describe changes below.
  • AI was used in preparing this PR. Please describe usage below.

@aidenwu23
aidenwu23 requested a review from veprbl August 23, 2026 22:44
@aidenwu23

aidenwu23 commented Aug 26, 2026

Copy link
Copy Markdown
Author

@veprbl I wasn’t able to run the benchmarks locally since they were taking quite a while. I believe it should just require swapping a few input collections. Would you be able to run them when you get a chance to look through the PRs?

@veprbl

veprbl commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

It took a while to get to this
Baseline vs New Results.pdf
and it looks like everything is well. The theta resolution in insert_neturon benchmark is broken. We can review and merge. @aidenwu23 would you be available to debug the benchmark?

@aidenwu23

aidenwu23 commented Sep 11, 2026

Copy link
Copy Markdown
Author

@veprbl Yes, merge sounds good. Good to have the insert geometry and reconstruction updates moving again. I’ll also start looking at the insert_neutron benchmark and update you if I find anything.

@veprbl

veprbl commented Sep 11, 2026

Copy link
Copy Markdown
Member

I wonder, do we need to adjust sampling fraction for the new geometry?

@aidenwu23

aidenwu23 commented Sep 11, 2026

Copy link
Copy Markdown
Author

Sampling fraction shouldn't need changing since layer thicknesses didn't change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@veprbl veprbl Awaiting requested review from veprbl

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

topic: calorimetry relates to calorimetry topic: forward

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aidenwu23 @veprbl